Skip to content

Conversation

@staskus
Copy link
Contributor

@staskus staskus commented Mar 17, 2025

Description

Updating Stripe Terminal SDK update 4.2.0.

Breaking Changes

SDK has had breaking changes in the API, some of which are affecting the Woo app:

  1. A single connectReader. It also moves the delegate setting from the method to the configuration

There is now a single connectReader call used for all connection methods. This replaces the previous methods: connectBluetoothReader, connectInternetReader, and connectLocalMobileReader.

  1. A change in how disconnects are reported

In my testing, replacing didReportUnexpectedReaderDisconnect with reader:didDisconnect: works. However, in practice, this method is only triggered 1 minute after the reader connection is lost (e.g. reader is turned off). As I understand, it happens because due to automatic reconnection attempts in the background.

Update: Auto reconnect on unexpected disconnect is now enabled by default for mobile and Tap to Pay readers.
Update: The method for handling reader disconnects has changed.
Removed terminal:didReportUnexpectedReaderDisconnect: from the SCPTerminalDelegate. Use reader:didDisconnect: to be informed of reader disconnects.

  1. A few other renamings

Most notably, appleBuiltIn and localMobile is now consistently called tapToPay. I will make the same change in Woo codebase for consistency:

BluetoothReaderDelegate has been renamed to MobileReaderDelegate.

Update: Renamed "local mobile" and "apple built in" to "Tap To Pay" in all SDK types, function names, and error codes to align with Stripe branding for this functionality.

  1. Payment method type is now strongly typed

Update: SCPPaymentIntentParameters and SCPSetupIntentParameters now keep payment method types as values of the SCPPaymentMethodType enum rather than strings.

Other possibly relevant changes

  1. New discovering state and enforcing a single discovery operation

We already have our own discovering state when starting the card reader process.

We also have logic that uses locks to make sure another discovery process is not starting or canceling. I'll test separately if this change allows us to remove the locks developed in this PR.

New: Added a new enum value discovering to SCPConnectionStatus to represent when discovery is running.
Update: Subsequent calls to SCPTerminal's discoverReaders:delegate:completion: cancel all previously queued discovery operations. Only one discovery operation can run at any given time; all other discovery attempts will fail with SCPErrorCanceledDueToIntegrationError.

  1. Changes in cancelation

I don't know if this is worth exploring. Have we been missing some flows where cancelation should be supported?

Update: SCPTerminal's confirmPaymentIntent:completion, confirmSetupIntent:completion, and confirmRefund:completion operations now return SCPCancelable's that allow you to cancel the operation in certain scenarios.

I tested payment cancelation and it continues to work.

Update: Calls to SCPTerminal's cancelPaymentIntent:completion or cancelSetupIntent:completion will now cancel ongoing operations related to the specified intent.

Steps to reproduce

Feel free to pick a few scenarios, especially the ones that were tested with a simulator (e.g. Interac payments). Also, one of the larger changes was done around reporting of unexpected disconnection, so it's good to test some these scenarios.

Testing information

I tested IPP scenarios on iPhone 14 Pro (17.7) and POS on iPad Air M2 (18.2). I also used a simulated card reader for scenarios that weren't easily reproducible.

IPP

Tasks UK US CA
Connection from Order payment flow
Connection from Payments menu
Connection of a reader with a mandatory update ✅ (simulator)
Connection of M2 reader N/A N/A
Connection of Chipper reader N/A N/A
Connection of WisePad 3 reader N/A
Connection from switched off state
Take a payment
Refund a payment
Take payment after the in-line connection process
Take payment with a previously-connected reader
Take TPP payment N/A
Refund TTP payment N/A
Take card-inserted payment
Take Interac payment N/A N/A ✅ (simulator)
Refund Interac payment N/A N/A ✅ (simulator)
Reader update: error message when attempted with low battery ✅ (simulator) ✅ (simulator) ✅ (simulator)
Reader update: successfully updates software and starts payment (in that flow) when completed ✅ (simulator)

POS

Tasks UK US
Connection of M2 reader N/A
Connection of Chipper reader N/A
Connection of WisePad 3 reader N/A
Connection from switched off state
Take payment after the in-line connection process
Take payment with a previously-connected reader
Reader update: error message when attempted with low battery ☑️ (simulator) ✅ (simulator)
Reader update: successfully updates software and starts payment (in that flow) when completed ☑️ ✅ (simulator)
Payment cancelations (going back, disconnecting. exiting POS)

Screenshots


  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

Reviewer (or Author, in the case of optional code reviews):

Please make sure these conditions are met before approving the PR, or request changes if the PR needs improvement:

  • The PR is small and has a clear, single focus, or a valid explanation is provided in the description. If needed, please request to split it into smaller PRs.
  • Ensure Adequate Unit Test Coverage: The changes are reasonably covered by unit tests or an explanation is provided in the PR description.
  • Manual Testing: The author listed all the tests they ran, including smoke tests when needed (e.g., for refactorings). The reviewer confirmed that the PR works as expected on all devices (phone/tablet) and no regressions are added.

@staskus staskus added type: task An internally driven task. feature: mobile payments Related to mobile payments / card present payments / Woo Payments. labels Mar 17, 2025
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Mar 17, 2025

WooCommerce iOS📲 You can test the changes from this Pull Request in WooCommerce iOS by scanning the QR code below to install the corresponding build.

App NameWooCommerce iOS WooCommerce iOS
Build Numberpr15377-0198905
Version22.0
Bundle IDcom.automattic.alpha.woocommerce
Commit0198905
App Center BuildWooCommerce - Prototype Builds #13428
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

staskus added 3 commits March 18, 2025 09:59
…connect

In Stripe Terminal SDK, the method for handling reader disconnects has changed. Removed terminal:didReportUnexpectedReaderDisconnect: from the SCPTerminalDelegate. Use reader:didDisconnect: to be informed of reader disconnects.
@staskus staskus added this to the 22.0 milestone Mar 18, 2025
@staskus staskus marked this pull request as ready for review March 18, 2025 11:34
@staskus staskus modified the milestones: 22.0, 22.1 Mar 21, 2025
@jaclync
Copy link
Contributor

jaclync commented Mar 25, 2025

👋 Apology for the delayed response, I started the review but haven't finished it and tested it yet. I will make sure to prioritize this Wednesday and finish the review by EOD Wednesday. Additional review(s) would also be helpful.

Copy link
Contributor

@jaclync jaclync left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking on this non-trivial SDK update, and the detailed PR description and testing scenarios! LGTM :shipit:

The tasks I tested (I have an Interac test card):

IPP

Tasks US CA
Connection from Order payment flow  
Connection from Payments menu  
Connection of a reader with a mandatory update ✅ (simulator)  
Connection of M2 reader N/A
Connection of Chipper reader N/A
Connection of WisePad 3 reader N/A
Connection from switched off state  
Take a payment  
Refund a payment  
Take payment after the in-line connection process  
Take payment with a previously-connected reader  
Take TPP payment N/A
Refund TTP payment N/A
Take card-inserted payment  
Take Interac payment N/A
Refund Interac payment N/A
Reader update: error message when attempted with low battery ✅ (simulator)  
Reader update: successfully updates software and starts payment (in that flow) when completed ✅ (simulator) ✅ with WisePad 3, the update took about 5 minutes

POS

Tasks US
Connection of M2 reader
Connection of Chipper reader
Connection of WisePad 3 reader N/A
Connection from switched off state
Take payment after the in-line connection process
Take payment with a previously-connected reader
Reader update: error message when attempted with low battery ✅ (simulator)
Reader update: successfully updates software and starts payment (in that flow) when completed ✅ (simulator)
Payment cancelations (going back, disconnecting. exiting POS)

@@ -0,0 +1,4 @@
public enum PaymentMethodType {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: maybe we can add a comment potentially with a link to the Stripe doc, as it seems to mirror a subset of StripeTerminal.PaymentMethodType?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. I will.

I borrowed the approach of CardReaderType and CardReaderType+Stripe, it's a similar type used in a similar way.

}

public func reader(_ reader: Reader, didDisconnect reason: DisconnectReason) {
connectedReadersSubject.send([])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wondering, does TTP delegate have a similar method to didDisconnect in the bluetooth reader delegate now that TerminalDelegate is removed? Or TTP doesn't really have a "disconnecting" state?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

Both TapToPayReaderDelegate and MobileReaderDelegate have didDisconnect. These delegates are both SCPReaderDelegate, which defines didDisconnect.

It means that when we set delegate to self, this method is shared by both TTP and Reader delegates.

BluetoothConnectionConfigurationBuilder(delegate: self, locationId: locationId)
TapToPayConnectionConfigurationBuilder(delegate: self, locationId: locationId)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation, makes sense now. I wasn't aware both delegates extend the same base delegate.

public struct CardPresentPaymentsConfiguration: Equatable {
public let countryCode: CountryCode
public let paymentMethods: [WCPayPaymentMethodType]
public let paymentMethods: [PaymentMethodType]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❓ Is WCPayPaymentMethodType still used? Is it only meant to be used for WooPayments by its name?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. It's used for wider WooPayments case. WCPayPaymentMethodType is used by Networking layer to represent payment charges. It contains more payment types than are supported by IPP. It looks like it's aprt of WCPayCharge object that is used at least for the refunds.

PaymentMethodType is meant to be used by Hardware layer and represents payment types that are supported by IPP. We only used WCPayPaymentMethodType here to later change this value to String. However, Stripe SDK now accepts an enum, so it makes sense to define one within Hardware and ask in collectPayment method.

@staskus staskus merged commit 4821ba5 into trunk Mar 27, 2025
13 checks passed
@staskus staskus deleted the task/stripe-sdk-update-4.2.0 branch March 27, 2025 08:46
@joshheald
Copy link
Contributor

Nice one on this @staskus – I read through it all and did some general testing, the changes look great to me.

@iamgabrielma
Copy link
Contributor

Sorry I'm late to the party: I was planning to test this earlier as part of my Beta testing rotation but I had to set myself AFK unexpectedly. I ran multiple of the cases above for IPP and POS using M2, as well as TTP cases and saw no issues 👍

Thanks for handling this @staskus

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature: mobile payments Related to mobile payments / card present payments / Woo Payments. type: task An internally driven task.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants